Skip to content

Doc: int -> int or Py_ssize_t#18663

Merged
methane merged 1 commit into
python:masterfrom
methane:carg-sizet
Feb 26, 2020
Merged

Doc: int -> int or Py_ssize_t#18663
methane merged 1 commit into
python:masterfrom
methane:carg-sizet

Conversation

@methane

@methane methane commented Feb 26, 2020

Copy link
Copy Markdown
Member

No description provided.

@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @methane for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

@bedevere-bot

Copy link
Copy Markdown

GH-18665 is a backport of this pull request to the 3.8 branch.

@bedevere-bot

Copy link
Copy Markdown

GH-18666 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 26, 2020
(cherry picked from commit 57c7a0b)

Co-authored-by: Inada Naoki <songofacandy@gmail.com>
miss-islington added a commit that referenced this pull request Feb 26, 2020
(cherry picked from commit 57c7a0b)

Co-authored-by: Inada Naoki <songofacandy@gmail.com>
miss-islington added a commit that referenced this pull request Feb 26, 2020
(cherry picked from commit 57c7a0b)

Co-authored-by: Inada Naoki <songofacandy@gmail.com>
@vstinner

Copy link
Copy Markdown
Member

I'm not sure if this change is correct: the function uses int or Py_ssize_t depending on #define PY_SSIZE_T_CLEAN.

Are you suggesting to stop supporting C extension which don't use PY_SSIZE_T_CLEAN?

See the note:

For all # variants of formats (s#, y#, etc.), the type of the length argument (int or Py_ssize_t) is controlled by defining the macro PY_SSIZE_T_CLEAN before including Python.h. If the macro was defined, length is a Py_ssize_t rather than an int. This behavior will change in a future Python version to only support Py_ssize_t and drop int support. It is best to always define PY_SSIZE_T_CLEAN.

https://docs.python.org/dev/c-api/arg.html

@methane

methane commented Apr 24, 2020

Copy link
Copy Markdown
Member Author

I'm not sure if this change is correct: the function uses int or Py_ssize_t depending on #define PY_SSIZE_T_CLEAN.

As far as I know, it is correct. All # format uses int or Py_ssize_t depending on PY_SSIZE_T_CLEAN.

Are you suggesting to stop supporting C extension which don't use PY_SSIZE_T_CLEAN?

Yes. We raise DeprecationWarning when # is used without PY_SSIZE_T_CLEAN now.
I want to remove int support in the future. But I'm not sure we can remove it in 3.10.

(cc @serhiy-storchaka )

@serhiy-storchaka

Copy link
Copy Markdown
Member

It is definitely an improvement. Maybe we could add a footnote in all these sites to clarify that the type is Py_ssize_t when PY_SSIZE_T_CLEAN is defined and int otherwise, but there is a large note documenting this above the list of format units.

As for removing int support, I think we should not haste. The first version with runtime warnings is not achieved even the beta stage. We may to make warnings more quiet or use other mechanisms to control them if they will be too annoying. 3.10 and even 3.11 is too early to remove int support. I think we should start this when versions older then 3.9 become unsupported. I.e. in 3.12, 3.13 or later.

@methane

methane commented Apr 24, 2020

Copy link
Copy Markdown
Member Author

The first version with runtime warnings is not achieved even the beta stage.

Python 3.8 implements the runtime warning. Python 3.9 will be the second version of runtime warning.
https://github.com/python/cpython/pull/12473/files#diff-77c703d9a958f6a4b0dc2f692b3fd5b3

Static checker will be better, but I don't know how we can achieve it.

@serhiy-storchaka

Copy link
Copy Markdown
Member

Sorry, I thought it is a 3.9 feature. In any case 3.10 is too earlier for breaking C API change. Especially with reducing the time between releases.

Maybe PyCharm or VS Code will start to complain about PyArg_Parse() without PY_SSIZE_T_CLEAN and provide refactoring tools to insert PY_SSIZE_T_CLEAN and convert size types automatically.

@vstinner

Copy link
Copy Markdown
Member

Python 3.8 implements the runtime warning

In that case, IMO it's fine to start to raise an error in Python 3.10: we would have two releases (3.8 and 3.9) with the deprecation warning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation in the Doc dir skip issue skip news

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants